Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 97.30%. Comparing base (24539fd) to head (6954605).

@@           Coverage Diff           @@
##           master     #406   +/-   ##
=======================================
  Coverage   97.30%   97.30%           
=======================================
  Files         117      117           
  Lines        6948     6951    +3     
=======================================
+ Hits         6761     6764    +3     
  Misses        187      187           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

• Flaky Tests Detection - Detect and resolve failed and flaky tests

May I ask what Julia version you used for these benchmarks? In the past this kind of optimization would often lead to significant speedups but from Julia v1.11 on, arrays are implemented differently: https://github.com/JuliaLang/julia/blob/v1.11.0/NEWS.md#new-language-features

I also think that perhaps the graphs you choose for benchmarking are kind of very small - for this kind of function we can also work with graphs with millions of vertices.
And so far I saw that you benchmarked with either paths or complete multipartite graphs - both are kind of pathological cases where either the search beam is very small or very wide - perhaps there is a more natural version somewhere in the middle?

This was on 1.10.5, so indeed before the Memory changes, good point.

The original version improves on 1.11 - but so does this revision (but the gap probably narrows a bit overall):

julia> g = path_graph(4)
julia> @b has_path($g, 1, 3) # true
50.041 ns (5 allocs: 224 bytes)
julia> @b new_has_path($g, 1, 3) # true
42.559 ns (5 allocs: 192 bytes)

julia> g = path_graph(10)
julia> @b has_path($g, 1, 10) # true
94.484 ns (5 allocs: 240 bytes)
julia> @b new_has_path($g, 1, 10) # true
77.722 ns (5 allocs: 256 bytes)

julia> g = path_graph(10)
julia> @b has_path($g, 1, 10) # true
100.112 ns (5 allocs: 240 bytes)
julia> @b new_has_path($g, 1, 10) # true
79.012 ns (5 allocs: 256 bytes)

julia> pp = SimpleGraph(30, vcat([p.fadjlist, [v .+ (15) for v in p.fadjlist]]...))
julia> @b has_path($pp, 1, 16) # false
203.519 ns (6 allocs: 624 bytes)
julia> @b new_has_path($pp, 1, 16) # false
148.489 ns (5 allocs: 432 bytes)

julia> g = path_graph(1000)
julia> @b has_path($g, 1, 1000)
5.148 μs (5 allocs: 1.250 KiB)
julia> @b new_has_path($g, 1, 1000)
5.498 μs (6 allocs: 9.000 KiB)

julia> @b has_path($g, 1, 3)
101.500 ns (5 allocs: 1.250 KiB)
julia> @b new_has_path($g, 1, 3)
202.632 ns (6 allocs: 9.000 KiB)

I also think that perhaps the graphs you choose for benchmarking are kind of very small - for this kind of function we can also work with graphs with millions of vertices.

Sure, that is an undeniable tradeoff. To me though, we are already instantiating an nv(g)-sized vector in seen, so this change is in line with that. Still, it's a tradeoff, no doubt.
(I don't feel strongly about this PR either way; I ended up factoring out some bits myself to reuse the buffers for my use-case, which is much faster)

And so far I saw that you benchmarked with either paths or complete multipartite graphs - both are kind of pathological cases where either the search beam is very small or very wide - perhaps there is a more natural version somewhere in the middle?

Also fair; I just don't know that many canonical/conventional graph kinds, so I just picked a few of the ones provided by Graphs.jl. Open to suggestions.

@thchr Can you rebase your PR on the lastest master? I think the nightly tests might succeed then.

Thanks for this suggestion, but I would rather weigh in against greedy allocation of a huge vector that we may never need. I think @simonschoelly is right and relying on Julia's compiler to get better is the right call

We could use an actual queue from DataStructures.jl instead of a vector though

Thanks for this suggestion, but I would rather weigh in against greedy allocation of a huge vector that we may never need. I think @simonschoelly is right and relying on Julia's compiler to get better is the right call

Okay, no worries; feel free to close.

We could use an actual queue from DataStructures.jl instead of a vector though

I think that is likely to be a good bit slower for small graphs, probably big ones too; the Queue type is quite complicated and allocates several arrays. For my use-case, this would almost certainly be substantially worse.